Skip to content

[pull] master from mattermost:master#705

Merged
pull[bot] merged 6 commits into
code:masterfrom
mattermost:master
May 14, 2026
Merged

[pull] master from mattermost:master#705
pull[bot] merged 6 commits into
code:masterfrom
mattermost:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull Bot commented May 14, 2026

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

hmhealey and others added 6 commits May 14, 2026 10:16
* Changed batchGetProfilesInChannel to batchGetProfilesInGroupChannel and have it use bulk API

* MM-65058 Add useUserIdsInGroupChannel and use to populate Direct Messages modal

* Address feedback

* Run Prettier

* Fix types
* Generalize file backend error types

Replace S3FileBackendAuthError and S3FileBackendNoBucketError with
backend-agnostic FileBackendAuthError and FileBackendNoBucketError so
non-S3 drivers can return them and the admin "Test Connection" flow
keeps surfacing useful messages.

The old S3-prefixed names are kept as type aliases of the generic
types so external code (plugins, historical consumers) continues to
compile, and so existing S3 construction sites stay untouched.

The type switch in connectionTestErrorToAppError now matches the
generic types, with new i18n keys (test_connection_auth.app_error
and test_connection_no_bucket.app_error) whose wording does not name
S3. The old S3-specific i18n keys are dropped via `make i18n-extract`
since they are no longer referenced from code; the api4 test that
asserted on those keys is updated, and the Cypress
`MM-T996 Amazon S3 connection error messaging` spec that asserted
on the old user-facing string is updated to the new wording.

------
AI assisted commit

* Pull in Azure SDK and uuid dependencies

Bring in github.com/Azure/azure-sdk-for-go/sdk/azcore and
.../sdk/storage/azblob (with .../sdk/internal as their indirect
dependency). The two are needed by the upcoming Azure Blob Storage
filestore backend and its lazy-Range-backed reader. The bump of
golang.org/x/{crypto,net,sys,term,text} comes transitively from
azblob's minimum versions.

Also promotes github.com/google/uuid from indirect to direct,
since the Azure backend uses it to generate block IDs that share
the same wire format the SDK itself produces in UploadStream.

------
AI assisted commit

* Add azureRangeReader, a seekable Range-backed blob reader

A small standalone type that satisfies the FileBackend interface's
ReadCloseSeeker + the broader io.ReaderAt contract on top of Azure
Blob Storage HTTP Range requests. Lands as its own commit because
the upcoming Azure FileBackend driver builds on it, and the reader
itself is independently useful — and independently testable against
a fake downloader without standing up an Azure client.

Design notes:

* Read opens an HTTP Range stream lazily at the current offset and
  reuses it for sequential reads. Seek to a different offset closes
  the open stream; the next Read re-opens it.
* Seek to the same offset is a no-op and does not close the open
  stream, so callers like zip.NewReader that probe with redundant
  seeks don't kick off a fresh download.
* ReadAt issues a dedicated ranged DownloadStream per call and does
  not touch the streaming cursor — matches the io.ReaderAt contract
  the bulk-import worker's zip.NewReader path relies on.
* Close cancels the context (which any in-flight Azure call will
  observe and abort), stops the deadline timer, and closes the
  current body if any. It is safe to call when no body was ever
  opened.
* CancelTimeout lets long-running consumers like the import worker
  opt out of the per-operation deadline that would otherwise kill
  multi-minute downloads partway through.

The implementation talks to a small blobDownloader interface rather
than *blob.Client directly so the unit tests can substitute a fake
downloader that records every requested Range and tracks Close
calls on the bodies it hands out.

------
AI assisted commit

* Add Azure Blob Storage filestore driver

Implements the FileBackend interface against Azure Blob Storage in
a new azurestore.go (~520 LOC). The driver is not yet selectable
via NewFileBackend's switch — that wiring lands in the next commit
together with the admin config surface — but the driver itself is
complete and self-contained behind the FileBackendSettings struct.

Filesstore.go grows three pieces of supporting infrastructure that
the driver consumes:

* a `driverAzure = "azureblob"` constant alongside the existing
  driverS3 and driverLocal,
* an Azure-specific block on FileBackendSettings (storage account,
  access key, container, path prefix, endpoint, SSL flag, request
  timeout),
* a CheckMandatoryAzureFields validator that mirrors
  CheckMandatoryS3Fields.

Behavioural notes that warrant calling out:

* Reader returns the previously-added azureRangeReader, so reads
  stream lazily over HTTP Range and ReadAt is available for the
  bulk-import worker's zip.NewReader path. The deadline timer is
  armed before the initial GetProperties call so the HEAD itself
  is bounded.
* WriteFile and AppendFile both go through StageBlock +
  CommitBlockList via a shared stageBlocks helper, never the SDK's
  UploadStream. UploadStream's small-payload fast path falls back
  to single-shot PutBlob, which leaves the resulting blob with no
  committed block list; a subsequent AppendFile that calls
  CommitBlockList on that blob would then clobber its content.
  Routing every write through the block-list mechanism keeps
  AppendFile correct regardless of payload size.
* AppendFile stages the new chunk as one or more blocks and commits
  the existing committed block list plus the newly staged IDs.
  The new bytes go up exactly once — no re-download, no
  re-concatenate, no re-upload of the prior contents.
* WriteFileContext does not wrap the caller-supplied context with
  its own timeout — that timeout is applied in WriteFile only,
  matching the S3 driver, so long-running TryWriteFileContext
  callers (like message-export bulk writes) opt out of the
  per-operation timeout the way the abstraction documents.

Authentication is shared-key only for this drop; Microsoft Entra
ID / managed identity is deferred to a follow-up. The endpoint is
configurable so the same code targets the production Azure host
(vhost style — {account}.blob.core.windows.net) or Azurite /
Azure Government / sovereign clouds (path style —
host[:port]/{account}).

------
AI assisted commit

* Wire Azure backend into config, validation, and driver selection

This commit registers the previously-added AzureFileBackend driver
with the rest of the system. Until now the driver was usable only
via direct construction; after this commit, `DriverName: "azureblob"`
in config.json is a fully-supported deployment configuration.

Five integration sites are touched:

* `newFileBackend` in filesstore.go now dispatches `driverAzure` to
  NewAzureFileBackend, alongside the existing s3 and local cases.
  NewFileBackendSettingsFromConfig (and its export counterpart) gain
  an Azure branch that maps the model.FileSettings fields onto the
  Azure-specific FileBackendSettings fields.
* `model.FileSettings` grows the user-facing Azure config schema:
  storage account, access key, container, path prefix, endpoint,
  SSL flag, request timeout, plus matching Export* fields for the
  dedicated export store. SetDefaults populates them so deployments
  that never opted into Azure don't carry nil pointers. `isValid`
  accepts the new ImageDriverAzure constant.
* `Config.Sanitize()` masks AzureAccessKey and ExportAzureAccessKey
  the same way it masks AmazonS3SecretAccessKey, so the shared key
  never reaches an API consumer in plain text.
* `desanitize()` restores the masked keys on a config write so a
  PATCH that doesn't touch the key doesn't clobber it with the
  FakeSetting placeholder.
* `configSensitivePaths` covers both Azure key paths so audit
  diffs don't include them either.
* `ConfigToFileBackendSettings` in the `mattermost db` CLI helper
  gets the Azure branch its production counterpart already has —
  without it, `mattermost db migrate` / `db downgrade` would fail
  on Azure-configured deployments with "missing azure storage
  account setting".

Finally, the shared FileBackendTestSuite is now wired against
Azurite via TestAzureFileBackendTestSuite, which skips when
CI_AZURITE_HOST is unreachable. The test-infra wiring (the docker
service, the env vars, the start_dependencies entry) landed in a
previous PR; this commit is what makes the suite actually exercise
the Azure driver end to end.

------
AI assisted commit

* Validate Azure timeout and path prefix in Config.IsValid

Parity with the S3-side checks that already cover
AmazonS3RequestTimeoutMilliseconds and AmazonS3PathPrefix. Without
these, a zero/negative AzureRequestTimeoutMilliseconds passes
validation and later creates immediately-expired request contexts,
and leading/trailing whitespace in AzurePathPrefix produces blob
keys that don't match what the admin configured.

Same checks added for the Export* counterparts. The
file_driver.app_error translation is updated to mention the new
'azureblob' option alongside 'local' and 'amazons3'.

------
AI assisted commit

* Stream zip entries from the Azure backend

writeZipEntry was calling ReadFile, which loads the entire blob
into memory before writing it to the archive. For large blobs or
deep directories this spikes RSS or OOMs the goroutine. Switch to
Reader (the streaming azureRangeReader) and io.Copy into the zip
entry so memory stays bounded regardless of blob size.

------
AI assisted commit

* Use a backend-agnostic fallback for FileBackendNoBucketError

The fallback Error() message was "no such bucket", which leaks S3
terminology when an Azure caller returns the type with no wrapped
Err. Use "no such bucket or container" so logs and external error
handling stay neutral across backends.

------
AI assisted commit

* Defend Azure path prefix against directory traversal

Reject ".." in AzurePathPrefix and ExportAzurePathPrefix at config
validation time, since path.Join collapses traversal segments and a
prefix like "../other-tenant" would otherwise escape the configured
isolation boundary.

Harden the prefix helper as a second line of defense: if the joined
path no longer sits inside pathPrefix, fall back to joining the prefix
with the base name of the caller-supplied path. That preserves the
prefix invariant for plugin and import paths that the upload code does
not sanitize uniformly.

------
AI assisted commit

* Honor SkipVerify when constructing the Azure client

FileBackendSettings.SkipVerify is plumbed through from the System Console
the same way it is for S3, so admins toggling the flag for self-signed
endpoints (Azurite, sovereign clouds) get the behavior they expect
without having to drop SSL entirely and send the shared key in clear
text.

------
AI assisted commit

* Warn when the Azure request timeout falls back to its default

Config.IsValid already rejects non-positive AzureRequestTimeoutMilliseconds
for any path that goes through config validation, so this warn only fires
for direct callers that bypass validation (tests, helpers). Logging the
substitution turns a silent coercion into something an operator can
correlate against unexpected request behavior.

------
AI assisted commit

* Cap Azure request timeout at 10 minutes

Reject AzureRequestTimeoutMilliseconds values above the ceiling so an
operator (or someone who has admin access) cannot effectively disable
timeouts by setting the value to math.MaxInt64. A hung Azure call then
holds a goroutine open until the OS gives up.

Applies the same bound to ExportAzureRequestTimeoutMilliseconds. S3 has
the same gap; treating it is out of scope here but worth a follow-up.

------
AI assisted commit

* Refuse AppendFile on blobs without a committed block list

A blob written by another tool (Azure portal, azcopy, a migration script,
a plugin using Put Blob) has its content in the blob but an empty
committed-block list. Committing a new block list against such a blob
silently replaces the existing content with only the appended bytes.

Check the blob's properties before staging when the committed-block list
is empty, and refuse with a clear error if the blob has content. Same
hazard for an admin pointing the backend at an existing container with
pre-existing files.

Adds an integration test against Azurite to lock the behavior in.

------
AI assisted commit

* Surface truncated reads from azureRangeReader

Read closed the body cleanly and returned io.EOF even when the remote
stream terminated before the blob's content length. Callers (and any
retry layer above) then accepted a partial blob as complete.

ReadAt unconditionally rewrote io.ErrUnexpectedEOF to io.EOF, which made
truncated downloads indistinguishable from clean reads. That is exactly
what zip.NewReader consumes for archive readers, so the bulk-import
worker would silently import partial archives.

Read now closes the body, nils it, and returns io.ErrUnexpectedEOF when
EOF arrives before offset reaches size. ReadAt only collapses
ErrUnexpectedEOF to EOF when the full count was delivered and the stream
was consumed to the end of the blob. Otherwise the truncation
propagates with context.

Both code paths are exercised by new fakeDownloader-backed tests.

------
AI assisted commit

* Move container provisioning out of Azure TestConnection

Auto-creating the container inside TestConnection meant a typo in the
System Console (mattermosst instead of mattermost) silently provisioned
an unwanted container in the admin's Azure subscription, with no audit
log and no warning. They'd discover it later when uploads landed
somewhere unexpected.

TestConnection now returns FileBackendNoBucketError when the container
is missing, mirroring the S3 contract. A new MakeContainer method
mirrors S3FileBackend.MakeBucket, and Server.Start dispatches via two
capability interfaces (bucketMaker / containerMaker) instead of a hard
S3 type assertion — so the NoBucket error is no longer silently
swallowed for backends Server.Start has not been taught about.

------
AI assisted commit

* Carry file backend auth detail through to AppError

The Test Connection button collapsed every typed backend failure into
the same generic i18n message. Operators trying to debug bad credentials
or a missing bucket only saw "Unable to authenticate against the file
storage backend" with no SDK code to grep for in their logs.

Use errors.As so the typed checks survive future wrapping, and pass the
underlying error string through the NewAppError details argument. The
AppError serializer surfaces that detail to the admin console alongside
the translated message, so a bad S3 InvalidAccessKeyId or an Azure
AuthenticationFailed shows up in the toast without an i18n schema
change.

------
AI assisted commit

* Remove non-ascii characters from comments

------
AI assisted commit

* Make linter happy

------
AI assisted commit

* Harden Azure prefix boundary check

strings.HasPrefix on the joined path is a string-level check, not a
path-level one, so a configured prefix of "mattermost" accepts a joined
result of "mattermost-evil/...". A crafted caller path like
"../mattermost-evil/secrets" would collapse via path.Join to that exact
sibling and slip through the boundary check, escaping the configured
prefix scope.

Require the joined path to be the cleaned prefix itself or to start with
the prefix followed by a path separator. The fallback path.Join uses the
same cleaned prefix for consistency.

------
AI assisted commit

* Provision Azurite container in standalone test setup

The shared FileBackendTestSuite's SetupTest already handles a missing
container by detecting FileBackendNoBucketError from TestConnection and
calling MakeContainer, but TestAzureFileBackendAppendRefusesNonBlockBlob
bypasses SetupTest and calls TestConnection directly. On a fresh Azurite
instance the test would fail before exercising the append-refusal logic.

Extract a newAzuriteBackend(t) helper alongside azuriteSettings(t) that
builds the backend and ensures the container exists, mirroring the
suite's setup. Use errors.As for forward compatibility with future
wrapping.

------
AI assisted commit

* Fix grammar in email-settings i18n string

"Email settings has unset values." -> "Email settings have unset values."

------
AI assisted commit

* Make Azure MakeContainer idempotent

Treat a ContainerAlreadyExists response as success so that two nodes
racing through TestConnection plus MakeContainer at boot both converge
instead of having the loser fail. Mirrors how the S3 backend handles
the equivalent BucketAlreadyOwnedByYou case.

------
AI assisted commit

* Narrow AzureEndpoint comment to path-style only

The setting only builds path-style URLs, so it cannot reach sovereign
clouds like Azure Government or Azure China, which require vhost-style
endpoints. Update the comment to reflect what the code actually does
and document that sovereign-cloud support is out of scope.

------
AI assisted commit
@pull pull Bot locked and limited conversation to collaborators May 14, 2026
@pull pull Bot added the ⤵️ pull label May 14, 2026
@pull pull Bot merged commit d43dbe9 into code:master May 14, 2026
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants